Skip to content

Constrain Agent Hook autofix reminders to exact reviews - #1056

Open
mariusvniekerk wants to merge 7 commits into
mainfrom
t3code/constrain-autofix-hook-scope
Open

Constrain Agent Hook autofix reminders to exact reviews#1056
mariusvniekerk wants to merge 7 commits into
mainfrom
t3code/constrain-autofix-hook-scope

Conversation

@mariusvniekerk

@mariusvniekerk mariusvniekerk commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Agent Hook reminders could repeatedly rediscover the same failed reviews and turn background feedback into unrelated repair work. The default fallback could also start a separate fix agent that had no reliable view of the user's active task.

Reminders now name the exact review jobs they surfaced and invoke only the bundled roborev-fix skill. The skill treats findings as claims to prove against current code: it fixes valid in-scope problems, closes invalid reviews with evidence and no code change, and leaves valid out-of-scope work open for the user. Customized instructions remain complete overrides.

Delivered job IDs are remembered per agent session and repository lineage so later hook events do not reopen the same loop; newly created jobs remain eligible. Observation stays best-effort because post-tool hooks run frequently. Hook installation also refreshes bundled skills for Claude Code, Codex, Factory Droid, and Grok Build. Profiles without a bundled variant deliberately receive no generic CLI fallback.

generated by a clanker

@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (b22fe79)

The changes need revision due to two medium-severity workflow issues.

Medium

  • Unavailable fix workflow for several hook profiles — internal/config/config.go:657
    Every hook profile is instructed to invoke roborev-fix, but that skill is installed only for Claude, Codex, Droid, and Grok. Copilot, Cursor, Gemini, Hermes, and Qwen receive an unavailable workflow without a CLI fallback, after which the review IDs are acknowledged and will not be surfaced again.
    Fix: Provide an exact-ID CLI workflow for profiles without bundled skills, or add compatible skill variants before enabling the instruction by default.

  • Legacy reminders retain the broad discovery command — internal/agenthook/state.go:824
    Deferred reminders without a persisted Instruction retain their previous Reason, which may include the old roborev fix --open --list workflow. Appending exact review IDs does not remove that broad command, so reminders queued before upgrading can still exceed the current task scope.
    Fix: Detect and replace the former built-in instruction during migration while preserving custom reasons, or safely discard legacy built-in reminders.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 5m9s

@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (471d833)

Code review found one medium-severity issue that could re-emit acknowledged reviews.

Medium

  • internal/agenthook/state.go:415 — Acknowledged review IDs are filtered before the final lineage key is determined. Detached-to-branch amend/rewrite handling may subsequently restore the prior lineage, causing already acknowledged reviews to be emitted again.
    • Fix: Compute actionableReviewIDs and failedReviewCount only after all lineage-key adjustments are complete.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 5m36s

@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (aa88918)

Review requires changes: one medium-severity compatibility issue affects several supported agent profiles.

Medium

  • Unavailable roborev-fix skill for supported profiles — internal/config/config.go:657, internal/agenthook/install.go:150
    The default instruction requires roborev-fix for every supported profile, but installation provides it only for Claude, Codex, Droid, and Grok. Fresh Copilot, Gemini, Hermes, Qwen, and Cursor installations receive reminders invoking an unavailable skill without an actionable CLI fallback. Provide bundled skills for every profile, or use a scoped exact-ID CLI fallback for profiles without skill support.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 5m52s

@mariusvniekerk

mariusvniekerk commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

The CLI Fallback is rejected since its a bad idea.

@mariusvniekerk mariusvniekerk self-assigned this Aug 17, 2026
@mariusvniekerk
mariusvniekerk force-pushed the t3code/constrain-autofix-hook-scope branch from aa88918 to 02f1fe0 Compare August 17, 2026 16:54
@roborev-ci

roborev-ci Bot commented Aug 17, 2026

Copy link
Copy Markdown

roborev: Combined Review (02f1fe0)

Upgrade migration is incomplete, leaving existing configs with outdated instructions.

Medium

  • internal/config/config.go:669 — Existing generated configs retain the previous default instruction, so upgraded profiles—especially those without bundled skills—continue allowing roborev fix --open --list. Migrate exact historical defaults in both hook config sections to the new default while preserving custom values.

Reviewers: 2 done | Synthesis: codex, 5s | Total: 5m27s

@mariusvniekerk
mariusvniekerk force-pushed the t3code/constrain-autofix-hook-scope branch from 02f1fe0 to d3d8224 Compare August 17, 2026 17:32
@roborev-ci

roborev-ci Bot commented Aug 17, 2026

Copy link
Copy Markdown

roborev: Combined Review (d3d8224)

Code is generally sound, but one compatibility gap remains for several hook profiles.

Medium

  • internal/config/config.go:670 — The default reminder requires roborev-fix, but Copilot, Cursor, Gemini, Hermes, and Qwen receive neither that bundled skill nor the previous CLI fallback. Their installed hooks therefore suggest a workflow they cannot execute. Provide a scope-safe exact-ID CLI fallback or install a compatible skill for every supported profile.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 2m49s

@roborev-ci

roborev-ci Bot commented Aug 17, 2026

Copy link
Copy Markdown

roborev: Combined Review (56a193d)

Code is generally sound, but rerun failures can be incorrectly suppressed.

Medium

  • internal/agenthook/state.go:1210 — Acknowledgements are keyed only by job ID. Because reruns reuse that ID while replacing the review output, a newly failed rerun is suppressed for the session and lineage. Key acknowledgements by review generation (for example, job ID plus FinishedAt or UpdatedAt) and test that failed reruns prompt again.

Reviewers: 2 done | Synthesis: codex, 5s | Total: 4m29s

mariusvniekerk and others added 7 commits August 18, 2026 14:54
@mariusvniekerk @codex
fix(agent-hook): keep automatic fixes in task scope
78e5fe8 
Agent Hook reminders can interrupt unrelated work and turn open findings into
an unintended task. Automatic fixes must preserve the user-chosen boundary
instead of treating the hook as broader authorization.

Out-of-scope or unclear findings now stay untouched until the user gives
direction, and reviews with deferred findings stay open. The scope guard also
applies when the main hook instruction is customized.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk @codex
fix(agent-hook): preserve custom instructions
f569862 
Agent Hook configuration is an explicit override. Appending built-in safety or
continuation text changes user intent and makes the custom setting misleading.

Keep scope, deferral, and resume guidance in the default only. A nonempty
resolved instruction now passes through unchanged, while an empty reason falls
back to the default.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk @codex
fix(agent-hook): constrain reminders to exact reviews
c5a1de8 
Automatic reminders could rediscover the same open reviews and turn an unrelated user task into a repeated repair loop. Delivered review IDs now define the reminder boundary for one agent session and repository lineage.

The bundled fix workflow must validate findings before editing, close disproved reviews without code changes, and defer valid work outside the active task. Hook installation refreshes supported bundled skills so the default instruction never needs the separate CLI agent fallback.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk @codex
fix(agent-hook): keep acknowledged reviews suppressed
1f95baa 
Attaching detached work to a branch and then amending it can restore the
detached work identity after review IDs have already been filtered against the
temporary branch identity. That stale result can present a delivered review as
new and restart the automatic reminder loop.

Filter reviews only after rewrite handling selects the final work identity, so
an acknowledgement follows the work it belongs to.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk @codex
style(skills): separate test declarations
7248967 
The combined skill-policy coverage left adjacent top-level test declarations.
Keep the file aligned with the repository's Go formatting rules.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk @codex
ci: retrigger CodeQL analysis
2bacc8d 
The previous Go analysis completed but its result upload failed during a GitHub service outage. Trigger a fresh analysis without changing repository content.

No code validation was run because this commit intentionally has no tree changes.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk @codex
test(agent-hook): use in-process review source
0a4c785 
The regular daemon now owns Agent Hook review lookup. The acknowledgement
regression test must exercise that in-process boundary instead of the removed
hook-to-daemon HTTP path.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk
mariusvniekerk force-pushed the t3code/constrain-autofix-hook-scope branch from 56a193d to 0a4c785 Compare August 18, 2026 19:06
@roborev-ci

roborev-ci Bot commented Aug 18, 2026

Copy link
Copy Markdown

roborev: Combined Review (0a4c785)

Code is generally sound, but rerun failures can be incorrectly suppressed after acknowledgment.

Medium

  • internal/agenthook/state.go:1240 — Acknowledgments are keyed only by job ID, which is reused across reruns. After a failure is acknowledged, subsequent failed reruns remain suppressed for the session. Track acknowledgment by attempt identity, such as job ID plus review ID or completion timestamp.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 3m51s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@mariusvniekerk mariusvniekerk

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mariusvniekerk